Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Virtual routing fix #224

Merged
merged 12 commits into from
Jan 3, 2018
Merged

Virtual routing fix #224

merged 12 commits into from
Jan 3, 2018

Conversation

DeMoorJasper
Copy link
Member

@DeMoorJasper DeMoorJasper commented Dec 11, 2017

potential fix for closes #211

@albinotonnina
Copy link
Contributor

You are on fire @DeMoorJasper! 🙌👏

Sent with GitHawk

Copy link
Member

@devongovett devongovett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add a test please?

@@ -45,6 +45,10 @@ class HTMLAsset extends Asset {
if (node.attrs) {
for (let attr in node.attrs) {
let elements = ATTRS[attr];
// link is not a file
if (node.tag === 'a' && node.attrs[attr].lastIndexOf('.') < 1) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should just ignore all absolute paths within HTML instead? so just check if the first character is /?

Copy link
Member Author

@DeMoorJasper DeMoorJasper Dec 21, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't this also break any absolute path require, (this has already been reported as a bug)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why you'd want to require an absolute path so this might be ok.

Copy link
Contributor

@brandon93s brandon93s left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Conflicts, too. 😬

@DeMoorJasper
Copy link
Member Author

Conflicts have been fixed @brandon93s

@DeMoorJasper DeMoorJasper changed the title Potential fix for virtual routing Virtual routing fix Jan 1, 2018
@devongovett devongovett merged commit e09575d into parcel-bundler:master Jan 3, 2018
@DeMoorJasper DeMoorJasper deleted the feature/linkChecking branch January 3, 2018 09:11
devongovett pushed a commit that referenced this pull request Oct 15, 2018
* run prettier

* potential fix

* fix relative paths

* switch to last index to still support relative paths on legit assets

* add test

* fix failing test
devongovett pushed a commit that referenced this pull request Oct 15, 2018
* run prettier

* potential fix

* fix relative paths

* switch to last index to still support relative paths on legit assets

* add test

* fix failing test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 Cannot resolve dependency when I use <a> tag with virtual path
4 participants